Skip to content

Conversation

@kiwina
Copy link
Contributor

@kiwina kiwina commented Jun 2, 2025

PR for WorkspaceTracker FileSystemWatcher Leak (WorkspaceTracker_45)

This PR addresses the FileSystemWatcher leak in the WorkspaceTracker class, as identified in leak-report/guaranteed/WorkspaceTracker_45.md.

Description

The WorkspaceTracker class creates a FileSystemWatcher in its registerListeners method and adds it to a disposables array. However, the dispose() method of WorkspaceTracker currently only clears internal timers and does not dispose of the items in the disposables array, including the FileSystemWatcher. This leads to a resource leak when a WorkspaceTracker instance is disposed.

This patch updates the dispose() method in WorkspaceTracker.ts to iterate over and dispose of all items in the this.disposables array, ensuring the FileSystemWatcher is properly cleaned up.

Related Bug Report

#4236

Changes

  • Modified src/integrations/workspace/WorkspaceTracker.ts:
    • Updated the dispose() method to include:
      this.disposables.forEach((d) => d.dispose());
      this.disposables = []; // Clear the array

How to Test

  1. Open a workspace in VS Code with the Roo Code extension active.
  2. Allow the WorkspaceTracker to initialize (e.g., by opening/closing files or tabs, or simply waiting a short period).
  3. Simulate the disposal of the WorkspaceTracker instance. This might involve:
    • Closing the Roo Code webview panel if the WorkspaceTracker is tied to its lifecycle.
    • If testable in isolation, directly calling dispose() on a WorkspaceTracker instance.
  4. Verify (e.g., through logging, or by checking for continued file system monitoring if possible) that the FileSystemWatcher created by the WorkspaceTracker is no longer active.

Important

Fixes resource leak in WorkspaceTracker by ensuring FileSystemWatcher and other disposables are properly disposed of.

  • Behavior:
    • Fixes resource leak in WorkspaceTracker by updating dispose() method in WorkspaceTracker.ts to dispose of all items in this.disposables array.
    • Ensures FileSystemWatcher is properly disposed of, preventing resource leaks.
  • Testing:
    • Verify FileSystemWatcher is inactive after WorkspaceTracker disposal by simulating disposal and checking for continued file system monitoring.

This description was created by Ellipsis for 08f92e2. You can customize this summary. It will automatically update as commits are pushed.

Closes: #4236

@kiwina kiwina requested review from cte and mrubens as code owners June 2, 2025 06:16
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Jun 2, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 2, 2025
@daniel-lxs
Copy link
Member

Thank you @kiwina
This looks good to me.

Unrelated:
It might be a good idea to check if the items typically stored in this.disposables are not expected to throw errors, if so it might be a good idea to handle this.disposables.forEach((d) => d.dispose()) differently to make sure all items are disposed.

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jun 2, 2025
@daniel-lxs daniel-lxs changed the title fix: auto patch for WorkspaceTracker_45 fix(WorkspaceTracker): Dispose FileSystemWatcher and other disposables to prevent resource leaks Jun 3, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 4, 2025
@mrubens mrubens merged commit f561208 into RooCodeInc:main Jun 4, 2025
18 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 4, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

fix: Undisposed FileSystemWatcher in WorkspaceTracker (WorkspaceTracker_45)

3 participants